Skip to content

[Feature] Configurable Tempo & Owner-Triggered Epochs#2638

Open
evgeny-s wants to merge 42 commits into
devnet-readyfrom
feature/dynamic-tempo
Open

[Feature] Configurable Tempo & Owner-Triggered Epochs#2638
evgeny-s wants to merge 42 commits into
devnet-readyfrom
feature/dynamic-tempo

Conversation

@evgeny-s
Copy link
Copy Markdown
Collaborator

@evgeny-s evgeny-s commented May 6, 2026

Description

Implementation of the specification.

Demo (non-technical): https://dynamic-tempo-non-technical.netlify.app/
Demo (technical): https://dynamic-tempo-technical.netlify.app/#1

Related Issue(s)

  • Closes #[issue number]

Type of Change

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • Documentation update
  • Other (please describe):

Breaking Change

Check the 11. Breaking changes & compatibility section.

Checklist

  • I have performed a self-review of my own code
  • I have commented my code, particularly in hard-to-understand areas
  • I have run ./scripts/fix_rust.sh to ensure my code is formatted and linted correctly
  • I have made corresponding changes to the documentation
  • My changes generate no new warnings
  • I have added tests that prove my fix is effective or that my feature works
  • New and existing unit tests pass locally with my changes
  • Any dependent changes have been merged and published in downstream modules

Screenshots (if applicable)

Please include any relevant screenshots or GIFs that demonstrate the changes made.

Additional Notes

Please provide any additional information or context that may be helpful for reviewers.

Simulations (miner rewards Events):

Trigger epoch:
image
Set tempo:
image

@evgeny-s evgeny-s added the skip-cargo-audit This PR fails cargo audit but needs to be merged anyway label May 6, 2026
@evgeny-s evgeny-s changed the title Dynamic tempo implementation [Feature] Configurable Tempo & Owner-Triggered Epochs May 6, 2026
Self::blocks_until_next_epoch(netuid, Self::get_tempo(netuid), current_block) == 0
let tempo = Self::get_tempo(netuid);
if tempo == 0 {
return false;
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This means that if tempo is set to 0, manual trigger or max cap of MAX_TEMPO will not work. Is this by design?

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, the previous implementation meant that if tempo is 0, we don't run tempo on this subnet.
We don't have this case in Mainnet, but we support it. There is still a possibility to set the tempo to 0 by the root.
So we support the same semantics.

/// stateful scheduler (period `tempo + 1`, anchored on `LastEpochBlock`). Used by the
/// admin-freeze-window predicate and external tooling. Returns `u64::MAX` when
/// `tempo == 0` (legacy defensive short-circuit).
pub fn blocks_until_next_epoch(netuid: NetUid, tempo: u16, block_number: u64) -> u64 {
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

"Period is tempo + 1: next firing at last + tempo + 1." comment and the code below is not anymore correct.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Updated the function name and the comment to avoid the confusion: ad7ba80


let now = Self::get_current_block_as_u64();

Tempo::<T>::insert(netuid, tempo);
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should this be a function since Tempo and LastEpochBlock are always updated together?

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added a helper function: df184e3

@evgeny-s
Copy link
Copy Markdown
Collaborator Author

Hey, @basfroman , we have a bunch of tests that failed on the SDK side.
The main reason is now we have a stateful calculation for the next epoch block, because the tempo is dynamic.
So, here you can just call the new runtime API - get_next_epoch_start_block.


/// Effective activity cutoff in blocks, derived from `ActivityCutoffFactorMilli` and `Tempo`.
/// `cutoff_blocks = (factor × tempo) / 1000`, clamped to ≥ 1.
pub fn get_activity_cutoff_blocks(netuid: NetUid) -> u64 {
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

should we use it to replace get_activity_cutoff everywhere? for instance in metagraph.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, because ActivityCutoff is deprecated and will be removed in the follow-up release.
Who can help with updating it?

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It is fine to remove the storage later. Actually, my question is for https://github.com/opentensor/subtensor/blob/devnet-ready/pallets/subtensor/src/rpc_info/metagraph.rs#L713, if we should use get_activity_cutoff_blocks to replace get_activity_cutoff? It will change the return type, from u16 to u64.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good catch, for me to update:

  • metagraph call
  • breaking changes section in the description, regarding the metagraph

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could you please mark ActivityCutoff as #[deprecated(note = "Some explanation")]? It will propagate to metadata.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added: be695df

Copy link
Copy Markdown
Collaborator Author

@evgeny-s evgeny-s May 25, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@l0r1s , BTW, it generates a lot of warnings with the deprecation notice in tests. But I planned to remove it in the follow-up PR. Are we ok to keep warnings?

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Updated Metagraph + for now commented the deprecation note, otherwise clippy is failing. We will remove it fully in the follow up PR: a8fee82

evgeny-s added 3 commits May 15, 2026 14:07
# Conflicts:
#	pallets/subtensor/src/utils/rate_limiting.rs
# Conflicts:
#	pallets/subtensor/src/coinbase/run_coinbase.rs
#	pallets/subtensor/src/macros/dispatches.rs
#	pallets/subtensor/src/macros/events.rs
#	pallets/subtensor/src/macros/hooks.rs
#	pallets/subtensor/src/tests/migration.rs
#	pallets/subtensor/src/tests/mod.rs
#	pallets/subtensor/src/utils/rate_limiting.rs
@github-actions github-actions Bot mentioned this pull request May 21, 2026
4 tasks
- Updated reveal/commit logic based on stateful epoch index
- Updated migration for CR-v2 storage item
@github-actions
Copy link
Copy Markdown
Contributor

🔄 AI review updated — Skeptic: VULNERABLE

Copy link
Copy Markdown
Contributor

@github-actions github-actions Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

AI review — see the sticky summary comment for the verdict and the inline comments below for specific findings.

// `(hash, commit_epoch, commit_block, _)` layout. Field 1 was the absolute
// `commit_block`; it becomes `commit_epoch` (legacy modulo epoch). Field 2
// keeps the absolute `commit_block` (used by the epoch column-mask).
let crv2_entries: Vec<_> = WeightCommits::<T>::iter().collect();
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[HIGH] Migration collects unbounded commit storage into Wasm memory

This migration still materializes every WeightCommits entry into a Vec before processing it. WeightCommits is live per-subnet/per-hotkey commit storage, so a large state can allocate beyond Wasm memory or exceed upgrade execution limits before the migration reaches its reads/writes accounting. Process entries in a streaming/bounded way, or split this into a bounded multi-block migration with persisted progress.

Comment thread runtime/src/lib.rs
SubtensorModule::get_subnet_account_id(netuid)
}

fn get_next_epoch_start_block(netuid: NetUid) -> Option<u64> {
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[HIGH] Runtime changes need a spec_version bump

This PR changes runtime storage, dispatchables, pallet config, a runtime migration, and a runtime API, but RuntimeVersion still has spec_version: 407. Without bumping spec_version, upgrade coordination can fail because nodes do not see this as a new runtime version. Pinned here because the unchanged version line is outside the diff; bump spec_version in VERSION for this runtime-affecting change.

@github-actions
Copy link
Copy Markdown
Contributor

🔄 AI review updated — Skeptic: VULNERABLE

…to make it cohesive with the emission distribution
Copy link
Copy Markdown
Contributor

@github-actions github-actions Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

AI review — see the sticky summary comment for the verdict and the inline comments below for specific findings.

// `(hash, commit_epoch, commit_block, _)` layout. Field 1 was the absolute
// `commit_block`; it becomes `commit_epoch` (legacy modulo epoch). Field 2
// keeps the absolute `commit_block` (used by the epoch column-mask).
let crv2_entries: Vec<_> = WeightCommits::<T>::iter().collect();
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[HIGH] Migration collects unbounded commit storage into Wasm memory

WeightCommits::<T>::iter().collect() materializes every CR-v2 commit entry across all subnets/accounts before rewriting them. This is user-scaled chain state, so a large live map can make the runtime migration exhaust Wasm memory or exceed block limits during on_runtime_upgrade. Stream the iterator and rewrite entries one at a time instead of collecting the full map first; account reads/writes incrementally inside the loop.

Comment thread runtime/src/lib.rs
SubtensorModule::get_subnet_account_id(netuid)
}

fn get_next_epoch_start_block(netuid: NetUid) -> Option<u64> {
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[HIGH] Runtime changes need a spec_version bump

This PR changes the runtime API surface and pallet runtime behavior, but runtime/src/lib.rs still reports spec_version: 407. Without a spec version bump, nodes can treat incompatible native/Wasm runtimes as equivalent and clients may miss the runtime upgrade. Bump spec_version for this runtime-affecting change.

@github-actions
Copy link
Copy Markdown
Contributor

🔄 AI review updated — Skeptic: VULNERABLE

Copy link
Copy Markdown
Contributor

@github-actions github-actions Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

AI review — see the sticky summary comment for the verdict and the inline comments below for specific findings.

// `(hash, commit_epoch, commit_block, _)` layout. Field 1 was the absolute
// `commit_block`; it becomes `commit_epoch` (legacy modulo epoch). Field 2
// keeps the absolute `commit_block` (used by the epoch column-mask).
let crv2_entries: Vec<_> = WeightCommits::<T>::iter().collect();
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[HIGH] Migration collects unbounded commit storage into Wasm memory

This migration materializes every WeightCommits entry before rewriting them. Because it runs from on_runtime_upgrade, the entire live CR-v2 commit set is allocated in Wasm memory in one block; WeightCommits cardinality is not bounded by this migration, so a sufficiently large live state can exhaust Wasm memory or exceed the upgrade block budget and stall/brick the runtime upgrade. Rewrite this as a bounded/streaming migration or split it into cursor-based chunks, and only mark the migration complete after all chunks are processed.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We have a few WeightCommits on mainnet, so we should be good.

@github-actions
Copy link
Copy Markdown
Contributor

🔄 AI review updated — Skeptic: VULNERABLE

Copy link
Copy Markdown
Contributor

@github-actions github-actions Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

AI review — see the sticky summary comment for the verdict and the inline comments below for specific findings.

// `(hash, commit_epoch, commit_block, _)` layout. Field 1 was the absolute
// `commit_block`; it becomes `commit_epoch` (legacy modulo epoch). Field 2
// keeps the absolute `commit_block` (used by the epoch column-mask).
let crv2_entries: Vec<_> = WeightCommits::<T>::iter().collect();
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[HIGH] Migration collects unbounded commit storage into Wasm memory

WeightCommits::<T>::iter().collect() materializes every CR-v2 commit entry before rewriting any of them. This runs inside on_runtime_upgrade; if live commit storage is large enough, the upgrade can exceed Wasm memory or block weight and fail the runtime upgrade. Rewrite entries in a streaming/chunked way instead of collecting the full storage map first.

@github-actions
Copy link
Copy Markdown
Contributor

🔄 AI review updated — Skeptic: VULNERABLE

Copy link
Copy Markdown
Contributor

@github-actions github-actions Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

AI review — see the sticky summary comment for the verdict and the inline comments below for specific findings.

// `(hash, commit_epoch, commit_block, _)` layout. Field 1 was the absolute
// `commit_block`; it becomes `commit_epoch` (legacy modulo epoch). Field 2
// keeps the absolute `commit_block` (used by the epoch column-mask).
let crv2_entries: Vec<_> = WeightCommits::<T>::iter().collect();
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[HIGH] Migration collects unbounded commit storage into Wasm memory

WeightCommits::<T>::iter().collect() materializes every CR-v2 commit queue in Wasm memory before the rewrite loop starts. WeightCommits is live runtime storage with no global bound across accounts/subnets, so a sufficiently large committed set can make the runtime upgrade exceed Wasm memory or block limits and abort the upgrade. Rewrite entries while streaming the iterator, or split this into a bounded/stepped migration that limits work and memory per block.

@github-actions
Copy link
Copy Markdown
Contributor

🔄 AI review updated — Skeptic: VULNERABLE

Copy link
Copy Markdown
Contributor

@github-actions github-actions Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

AI review — see the sticky summary comment for the verdict and the inline comments below for specific findings.

// `(hash, commit_epoch, commit_block, _)` layout. Field 1 was the absolute
// `commit_block`; it becomes `commit_epoch` (legacy modulo epoch). Field 2
// keeps the absolute `commit_block` (used by the epoch column-mask).
let crv2_entries: Vec<_> = WeightCommits::<T>::iter().collect();
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[HIGH] Migration collects unbounded commit storage into Wasm memory

WeightCommits::<T>::iter().collect() loads every live CR-v2 commit row into one Vec during on_runtime_upgrade. This storage is keyed by subnet/account and can scale with live users, so a sufficiently large state can exhaust Wasm memory or trap before HasMigrationRun is written, blocking the runtime upgrade. Rewrite the storage incrementally with a bounded/paged migration state, or otherwise avoid collecting the full map before processing.

@github-actions
Copy link
Copy Markdown
Contributor

🔄 AI review updated — Skeptic: VULNERABLE

# Conflicts:
#	pallets/subtensor/src/macros/hooks.rs
#	pallets/subtensor/src/tests/migration.rs
#	runtime/src/lib.rs
Copy link
Copy Markdown
Contributor

@github-actions github-actions Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

AI review — see the sticky summary comment for the verdict and the inline comments below for specific findings.

// `(hash, commit_epoch, commit_block, _)` layout. Field 1 was the absolute
// `commit_block`; it becomes `commit_epoch` (legacy modulo epoch). Field 2
// keeps the absolute `commit_block` (used by the epoch column-mask).
let crv2_entries: Vec<_> = WeightCommits::<T>::iter().collect();
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[HIGH] Migration collects unbounded commit storage into Wasm memory

This runtime migration still loads every WeightCommits entry into a Vec before rewriting them. WeightCommits is live, user-influenced commit-reveal storage, so a sufficiently large state can make the upgrade allocate unbounded Wasm memory and abort the runtime upgrade. Iterate and rewrite entries incrementally instead of collecting the full map first, or otherwise bound/chunk the migration so the upgrade cannot OOM.

@github-actions
Copy link
Copy Markdown
Contributor

🔄 AI review updated — Skeptic: VULNERABLE

Copy link
Copy Markdown
Contributor

@github-actions github-actions Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

AI review — see the sticky summary comment for the verdict and the inline comments below for specific findings.

// `(hash, commit_epoch, commit_block, _)` layout. Field 1 was the absolute
// `commit_block`; it becomes `commit_epoch` (legacy modulo epoch). Field 2
// keeps the absolute `commit_block` (used by the epoch column-mask).
let crv2_entries: Vec<_> = WeightCommits::<T>::iter().collect();
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[HIGH] Migration collects unbounded commit storage into Wasm memory

WeightCommits::<T>::iter().collect() loads every CR-v2 commit entry into a single Wasm heap allocation before rewriting anything. This storage is user-populated across netuids/accounts, so a runtime upgrade can OOM/trap before HasMigrationRun is written, blocking the upgrade. Rewrite entries in a streaming or bounded way, for example by iterating keys and updating each entry immediately, or split the conversion into a cursor-based migration with bounded per-block work and accurate weight accounting.

@github-actions
Copy link
Copy Markdown
Contributor

🔄 AI review updated — Skeptic: VULNERABLE

Copy link
Copy Markdown
Contributor

@github-actions github-actions Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

AI review — see the sticky summary comment for the verdict and the inline comments below for specific findings.

// `(hash, commit_epoch, commit_block, _)` layout. Field 1 was the absolute
// `commit_block`; it becomes `commit_epoch` (legacy modulo epoch). Field 2
// keeps the absolute `commit_block` (used by the epoch column-mask).
let crv2_entries: Vec<_> = WeightCommits::<T>::iter().collect();
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[HIGH] Migration collects unbounded commit storage into Wasm memory

WeightCommits::<T>::iter().collect() materializes every CR-v2 commit entry across all subnets/accounts into Wasm memory during the runtime upgrade before rewriting them. This storage is live and user-populated, so the migration can exceed block/Wasm memory limits and brick the upgrade path. Process the map in a bounded streaming/drain/translate pattern, or otherwise prove an enforced upper bound that fits the runtime-upgrade budget.

@github-actions
Copy link
Copy Markdown
Contributor

🔄 AI review updated — Skeptic: VULNERABLE

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

breaking-change This PR introduces a noteworthy breaking change skip-cargo-audit This PR fails cargo audit but needs to be merged anyway

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants